gh-145040: Fix crash in sqlite3 when connection is closed from within a callback by raminfp · Pull Request #145041 · python/cpython

raminfp · 2026-02-20T15:42:22Z

Summary

Fix a segmentation fault (NULL pointer dereference) in the _sqlite module that occurs when Connection.close() is called inside an aggregate step() callback.

Fix

Primary check – immediately after stmt_step(), check if self->connection->db == NULL. If so, raise ProgrammingError("Cannot operate on a closed database.") and jump to the error path.
Defensive check – guard the sqlite3_last_insert_rowid() call with self->connection->db != NULL as an extra safety net.

Test

Added test_aggr_close_conn_in_step in Lib/test/test_sqlite3/test_userfunctions.py that reproduces the exact crash scenario and verifies that ProgrammingError is raised instead of a segfault.

ASAN output (before fix)

AddressSanitizer: SEGV on unknown address 0x000000000038
  #0 in sqlite3_last_insert_rowid
  #1 in _pysqlite_query_execute Modules/_sqlite/cursor.c:974

After fix

sqlite3.ProgrammingError: Cannot operate on a closed database.

Issue: _sqlite: NULL dereference when connection is closed from within a callback #145040

…g aggregate callback Fix a segmentation fault in the _sqlite module that occurs when Connection.close() is called inside an aggregate step() callback. After stmt_step() returns, _pysqlite_query_execute() calls sqlite3_last_insert_rowid(self->connection->db) without checking if self->connection->db is still valid. If the connection was closed during the callback, self->connection->db is NULL, causing a NULL pointer dereference. The fix adds a NULL check for self->connection->db after stmt_step() returns, raising ProgrammingError instead of crashing.

Misc/NEWS.d/next/Library/2026-02-20-00-00-00.gh-issue-145040.sqlite3-close-crash.rst

…qlite3-close-crash.rst Co-authored-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>

Modules/_sqlite/cursor.c

erlend-aasland · 2026-02-20T18:43:44Z

Please update the PR title to more accurately reflect the problem it addresses. Ditto for the NEWS item.

Modules/_sqlite/cursor.c

…within a callback

raminfp · 2026-02-21T00:20:37Z

Sorry for force push!!!

Lib/test/test_sqlite3/test_userfunctions.py

erlend-aasland · 2026-02-26T23:29:22Z

This PR adds a sanity check after the damage has been done. The SQLite C API clearly tells us what's legal and illegal operations on the sqlite3 * pointer during these callbacks. Consider an alternative approach: how about preventing the API misuse in the first place? That would probably require adding a flag to the connection object, and some machinery for setting/reading it, but it could possibly make for a better experience for the developer. I'm not sure the complexity would be worth it, though. WDYT?

…lback Instead of detecting a closed connection after the damage has been done, prevent Connection.close() from succeeding while a SQLite callback is executing. This aligns with the SQLite C API docs, which state that applications must not close the database connection from within a callback. Add an in_callback counter to the connection object, incremented before stmt_step() and decremented after. If close() is called while the counter is positive, ProgrammingError is raised and the database connection remains open. A counter (rather than a boolean flag) is used to correctly handle nested callbacks. Also convert test docstrings to comments per reviewer feedback, and add a test for the nested callback scenario.

raminfp · 2026-02-27T05:29:32Z

I agree, preventing the misuse upfront is the better approach. I've pushed a new commit that does exactly that:

Added an in_callback counter to the connection object, incremented/decremented around stmt_step() calls.
Connection.close() now checks this counter and raises ProgrammingError before touching the database, keeping the sqlite3 * pointer valid.
Used a counter (not a boolean) to correctly handle nested callbacks (e.g a UDF that triggers another query).

The complexity turned out to be quite manageable, just a few lines in close(), and ++/-- around the two stmt_step() call sites.

…ted callbacks Only check and consume the close_attempted_in_callback flag when in_callback reaches zero (the outermost level). Previously, a nested stmt_step() inside a callback could consume the flag, causing the outermost caller to miss the error.

erlend-aasland · 2026-02-27T21:46:06Z

We should be able to exploit the already present cursor->locked member of pysqlite_Cursor, @raminfp. This flag is set during the complete query orchestration and during the iter implementation. Unfortunately, the list of created cursors were recently purged in #144378, so we have no way of checking if a cursor is locked from within the connection close() method.

erlend-aasland · 2026-02-27T22:14:16Z

We should be able to exploit the already present cursor->locked member of pysqlite_Cursor […]

I did a quick PoC, and this indeed works. It's a small change, and it works for both the iternext and the query execute scenarios. However, it requires we roll back parts of 74c1f41. If we choose this path, we should do the rolling back as a separate PR first.

raminfp · 2026-02-28T02:32:49Z

@erlend-aasland I implemented your suggested approach. Here's what I did:

Removed in_callback counter, now using cursor->locked instead:

Rolled back parts of 74c1f41 - restored self->cursors weakref list and register_cursor() (without the old created_cursors counter / _pysqlite_drop_unused_cursor_references cleanup).
close() now iterates the cursor list and checks cursor->locked instead of checking in_callback > 0.
Extended locked region in cursor_iternext to cover stmt_step() - previously locked was only set during _pysqlite_fetch_one_row(), not during stmt_step() where callbacks actually fire.
Kept close_attempted_in_callback flag - this is still needed because when close() raises ProgrammingError inside a callback, SQLite's callback wrapper clears the Python exception and converts it to OperationalError. The flag lets us re-raise the correct ProgrammingError after stmt_step() returns.
Added any_other_cursor_locked() helper - for nested callback scenarios (e.g. test_close_conn_in_nested_callback_caught), we only clear close_attempted_in_callback at the outermost cursor level.
Added stmt_reset() before Py_CLEAR(self->statement) in iternext - when stmt_step is interrupted by a progress handler, the statement stays busy. Without resetting first, sqlite3_finalize during shutdown would crash.

All 509 test_sqlite3 tests pass.

…tion in callback Replace the in_callback counter with cursor->locked checks. Restore the cursor weakref list (partially rolling back 74c1f41) so that close() can iterate cursors and detect locked ones.

erlend-aasland · 2026-03-02T15:07:37Z

@raminfp, great. I'll be able to take a look at this later today (CET).

erlend-aasland · 2026-03-02T15:09:14Z

Modules/_sqlite/connection.c

 #include "pycore_pyerrors.h"      // _PyErr_ChainExceptions1()
 #include "pycore_pylifecycle.h"   // _Py_IsInterpreterFinalizing()
 #include "pycore_unicodeobject.h" // _PyUnicode_AsUTF8NoNUL
+#include "pycore_weakref.h"


I don't think we need pycore_weakref.h; we should be fine with the public API functions here.

See blob.c for how to iterate through the weak refs using the public API.

erlend-aasland · 2026-03-02T15:13:07Z

Modules/_sqlite/connection.c

    self->statement_cache = statement_cache;
+    self->cursors = cursors;
    self->blobs = blobs;
+    self->close_attempted_in_callback = 0;


Instead of adding this flag, did you try to just raise in pysqlite_connection_close_impl?

(The exception should propagate through the existing (slightly weird) callback exception handling in connection.c)

raminfp · 2026-03-02T20:02:35Z

@erlend-aasland

Thank you, now in Iran, the internet is cut off for us people. netblocks. I use DNS tunnel is very slow.

We are now in a crucial period in our country's history for freedom.

Please give me time to make these changes after freedom.

erlend-aasland · 2026-03-03T14:54:39Z

@raminfp, stay safe! Don't worry about the PR; it can wait.

raminfp requested review from berkerpeksag and erlend-aasland as code owners February 20, 2026 15:42

bedevere-app bot added the awaiting review label Feb 20, 2026

bedevere-app bot mentioned this pull request Feb 20, 2026

_sqlite: NULL dereference when connection is closed from within a callback #145040

Open

benediktjohannes reviewed Feb 20, 2026

View reviewed changes

Misc/NEWS.d/next/Library/2026-02-20-00-00-00.gh-issue-145040.sqlite3-close-crash.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Library/2026-02-20-00-00-00.gh-issue-145040.s…

1ecb567

…qlite3-close-crash.rst Co-authored-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>

erlend-aasland reviewed Feb 20, 2026

View reviewed changes

Modules/_sqlite/cursor.c Outdated Show resolved Hide resolved

erlend-aasland added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Feb 20, 2026

erlend-aasland self-assigned this Feb 20, 2026

erlend-aasland reviewed Feb 20, 2026

View reviewed changes

Modules/_sqlite/cursor.c Outdated Show resolved Hide resolved

raminfp changed the title ~~gh-145040: Fix crash in sqlite3 when connection is closed during aggregate callback~~ gh-145040: Fix crash in sqlite3 when connection is closed from within a callback Feb 21, 2026

pythongh-145040: Fix crash in sqlite3 when connection is closed from …

05086f0

…within a callback

raminfp force-pushed the fix-gh-145040-sqlite3-close-in-callback branch from 4305a82 to 05086f0 Compare February 21, 2026 00:19

raminfp requested a review from erlend-aasland February 21, 2026 00:22

Fix NEWS: use literal markup for sqlite3_step()

c7dbc7e

erlend-aasland reviewed Feb 26, 2026

View reviewed changes

Lib/test/test_sqlite3/test_userfunctions.py Outdated Show resolved Hide resolved

raminfp requested a review from erlend-aasland February 27, 2026 05:29

raminfp added 2 commits February 28, 2026 06:02

pythongh-145040: Use cursor->locked to prevent closing sqlite3 connec…

36ea504

…tion in callback Replace the in_callback counter with cursor->locked checks. Restore the cursor weakref list (partially rolling back 74c1f41) so that close() can iterate cursors and detect locked ones.

pythongh-145040: Close connections in tests to avoid ResourceWarning

c7707be

erlend-aasland reviewed Mar 2, 2026

View reviewed changes

Uh oh!

Conversation

raminfp commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Test

ASAN output (before fix)

After fix

Uh oh!

Uh oh!

Uh oh!

erlend-aasland commented Feb 20, 2026

Uh oh!

Uh oh!

raminfp commented Feb 21, 2026

Uh oh!

Uh oh!

erlend-aasland commented Feb 26, 2026

Uh oh!

raminfp commented Feb 27, 2026

Uh oh!

erlend-aasland commented Feb 27, 2026

Uh oh!

erlend-aasland commented Feb 27, 2026

Uh oh!

raminfp commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland commented Mar 2, 2026

Uh oh!

erlend-aasland Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

raminfp commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

raminfp commented Feb 20, 2026 •

edited

Loading

raminfp commented Feb 28, 2026 •

edited

Loading

raminfp commented Mar 2, 2026 •

edited

Loading